Skip to content

MINOR: add SCRAM for security_rolling_upgrade #18368

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 3 commits into
base: trunk
Choose a base branch
from
Open

MINOR: add SCRAM for security_rolling_upgrade #18368

wants to merge 3 commits into from

Conversation

TaiJuWu
Copy link
Collaborator

@TaiJuWu TaiJuWu commented Jan 1, 2025

As title

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@github-actions github-actions bot added triage PRs from the community tests Test fixes (including flaky tests) small Small PRs labels Jan 1, 2025
@TaiJuWu TaiJuWu marked this pull request as draft January 1, 2025 03:20
@TaiJuWu
Copy link
Collaborator Author

TaiJuWu commented Jan 1, 2025 

----------------------------------------------------------------------------------------------------
test_id:    kafkatest.tests.core.security_rolling_upgrade_test.TestSecurityRollingUpgrade.test_rolling_upgrade_sasl_mechanism_phase_one.new_client_sasl_mechanism=PLAIN.metadata_quorum=ISOLATED_KRAFT
status:     PASS
run time:   6 minutes 13.083 seconds
----------------------------------------------------------------------------------------------------
test_id:    kafkatest.tests.core.security_rolling_upgrade_test.TestSecurityRollingUpgrade.test_rolling_upgrade_sasl_mechanism_phase_one.new_client_sasl_mechanism=SCRAM-SHA-256.metadata_quorum=ISOLATED_KRAFT
status:     PASS
run time:   4 minutes 18.401 seconds
----------------------------------------------------------------------------------------------------
test_id:    kafkatest.tests.core.security_rolling_upgrade_test.TestSecurityRollingUpgrade.test_rolling_upgrade_sasl_mechanism_phase_one.new_client_sasl_mechanism=SCRAM-SHA-512.metadata_quorum=ISOLATED_KRAFT
status:     PASS
run time:   5 minutes 17.883 seconds
----------------------------------------------------------------------------------------------------

@TaiJuWu TaiJuWu marked this pull request as ready for review January 1, 2025 06:23
@TaiJuWu TaiJuWu marked this pull request as draft January 1, 2025 06:26
@TaiJuWu TaiJuWu marked this pull request as ready for review January 1, 2025 08:06
@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 8, 2025

A label of 'needs-attention' was automatically added to this PR in order to raise the
attention of the committers. Once this issue has been triaged, the triage label
should be removed to prevent this automation from happening again.

chia7712
chia7712 reviewed Feb 24, 2025
View reviewed changes
tests/kafkatest/services/kafka/kafka.py
@@ -842,8 +842,7 @@ def start_node(self, node, timeout_sec=60, **kwargs):
self.close_port(controller_listener)

self.security_config.setup_node(node)
if self.quorum_info.using_zk or self.quorum_info.has_brokers: # TODO: SCRAM currently unsupported for controller quorum
self.maybe_setup_broker_scram_credentials(node)
self.maybe_setup_broker_scram_credentials(node)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this method will call maybe_setup_broker_scram_credentials to use zk connection. Is it safe if the e2e is under kraft?

@github-actions github-actions bot removed needs-attention triage PRs from the community labels Feb 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chia7712 chia7712 chia7712 left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
small Small PRs tests Test fixes (including flaky tests)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@TaiJuWu @chia7712